Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduce a new LinkedUser base and Admin model plus ProgramAdmin through-model; migrate existing program-admin data; switch Program.admins to use Admin/ProgramAdmin; update Mentor to inherit LinkedUser; add admin UIs/inlines; adjust GraphQL nodes, API queries/mutations, migrations, and a local Docker volume name. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/apps/mentorship/api/internal/mutations/module.py (2)
224-237: Docstring is inconsistent with implementation.The docstring states "User must be a mentor and an admin of the program" but the code only checks if the user is a mentor of the module. Either update the docstring to match the implementation, or add the admin check.
🔎 Proposed docstring fix
def set_task_deadline( ... ) -> ModuleNode: - """Set a deadline for a task. User must be a mentor and an admin of the program.""" + """Set a deadline for a task. User must be a mentor of this module."""
283-296: Docstring is inconsistent with implementation.Same issue as
set_task_deadline: the docstring claims "User must be a mentor and an admin of the program" but only the mentor check is performed.🔎 Proposed docstring fix
def clear_task_deadline( ... ) -> ModuleNode: - """Clear the deadline for a task. User must be a mentor and an admin of the program.""" + """Clear the deadline for a task. User must be a mentor of this module."""
🧹 Nitpick comments (3)
backend/apps/mentorship/admin/module.py (1)
9-15: Consider exposing additional through-model fields.The inline currently only shows the
mentorfield. Based on the MentorModule model (which includesrole, experience level fields, and matching attributes), you may want to expose additional fields in the inline for more complete management.🔎 Consider this enhancement to expose more fields
class MentorModuleInline(admin.TabularInline): """Inline admin for MentorModule through model.""" model = MentorModule extra = 1 - fields = ("mentor",) + fields = ("mentor", "role") autocomplete_fields = ("mentor",)Or remove the
fieldsattribute entirely to show all fields from the MentorModule model.backend/apps/mentorship/api/internal/queries/program.py (1)
71-75: N+1 query on admin check inside loop.
program.admins.filter(id=user.id).exists()executes a database query for each program in the paginated results. Consider prefetching or computing admin status from the already-prefetchedadmins__github_userdata, or use a set of admin program IDs computed once before the loop.🔎 Proposed optimization
+ admin_program_id_set = set(admin_program_ids) + results = [] for program in paginated_programs: - is_admin = program.admins.filter(id=user.id).exists() + is_admin = program.id in admin_program_id_set program.user_role = "admin" if is_admin else "mentor" results.append(program)backend/apps/mentorship/models/program_admin.py (1)
16-19: Consider usingUniqueConstraintinstead ofunique_together.
unique_togetheris considered a legacy option. Django recommends usingconstraintswithUniqueConstraintfor new code, which also supports additional features like condition-based constraints.🔎 Proposed modernization
class Meta: db_table = "mentorship_program_admins" verbose_name_plural = "Program admins" - unique_together = ("program", "user") + constraints = [ + models.UniqueConstraint(fields=["program", "user"], name="unique_program_user_admin"), + ]
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/moduleQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/programsMutations.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/programsQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (10)
backend/apps/mentorship/admin/module.pybackend/apps/mentorship/admin/program.pybackend/apps/mentorship/api/internal/mutations/module.pybackend/apps/mentorship/api/internal/mutations/program.pybackend/apps/mentorship/api/internal/nodes/program.pybackend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.pybackend/apps/mentorship/models/__init__.pybackend/apps/mentorship/models/program.pybackend/apps/mentorship/models/program_admin.py
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:41-44
Timestamp: 2025-07-08T17:24:36.501Z
Learning: In the mentorship program GraphQL mutations, date validation is handled at the GraphQL schema/node level in the input types (CreateProgramInput, UpdateProgramInput), preventing null values from reaching the mutation logic where date comparisons are performed.
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/module.py:28-29
Timestamp: 2025-07-16T13:49:58.648Z
Learning: In the OWASP Nest mentorship system, mentors can be created with only github_user initially (without nest_user) when assigned to modules. This allows mentors to be assigned before they've signed into the system. When these users eventually sign in, the nest_user is automatically linked to the existing github_user, creating a complete mentor profile. This design provides flexibility in mentor assignment workflows.
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/mentorship/admin/program.pybackend/apps/mentorship/admin/module.pybackend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/models/__init__.pybackend/apps/mentorship/models/program_admin.pybackend/apps/mentorship/api/internal/mutations/module.pybackend/apps/mentorship/models/program.pybackend/apps/mentorship/api/internal/mutations/program.pybackend/apps/mentorship/api/internal/nodes/program.pybackend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/mentorship/admin/program.pybackend/apps/mentorship/admin/module.pybackend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/models/__init__.pybackend/apps/mentorship/models/program_admin.pybackend/apps/mentorship/api/internal/mutations/module.pybackend/apps/mentorship/models/program.pybackend/apps/mentorship/api/internal/mutations/program.pybackend/apps/mentorship/api/internal/nodes/program.pybackend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.py
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
backend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/api/internal/mutations/program.py
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
backend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/api/internal/mutations/program.py
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
backend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-07-13T05:55:46.436Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Applied to files:
backend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/api/internal/mutations/program.pybackend/apps/mentorship/api/internal/nodes/program.py
📚 Learning: 2025-07-16T13:49:58.648Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/module.py:28-29
Timestamp: 2025-07-16T13:49:58.648Z
Learning: In the OWASP Nest mentorship system, mentors can be created with only github_user initially (without nest_user) when assigned to modules. This allows mentors to be assigned before they've signed into the system. When these users eventually sign in, the nest_user is automatically linked to the existing github_user, creating a complete mentor profile. This design provides flexibility in mentor assignment workflows.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-07-14T16:18:07.287Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/components/ModuleForm.tsx:112-134
Timestamp: 2025-07-14T16:18:07.287Z
Learning: In the OWASP Nest mentorship system, date validation for modules (ensuring start date precedes end date) is handled on the backend in the module GraphQL mutations via the `_validate_module_dates` helper function in backend/apps/mentorship/graphql/mutations/module.py, which prevents invalid date ranges from being stored in the database.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-08-17T11:55:55.990Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2000
File: backend/apps/mentorship/management/commands/sync_module_issues.py:109-145
Timestamp: 2025-08-17T11:55:55.990Z
Learning: In the OWASP/Nest mentorship system, tasks are designed to be assigned to only one assignee per issue, even if GitHub issues can have multiple assignees. The sync_module_issues command correctly uses issue.assignees.first() to create one task per issue for the first assignee only.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-07-08T17:24:36.501Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:41-44
Timestamp: 2025-07-08T17:24:36.501Z
Learning: In the mentorship program GraphQL mutations, date validation is handled at the GraphQL schema/node level in the input types (CreateProgramInput, UpdateProgramInput), preventing null values from reaching the mutation logic where date comparisons are performed.
Applied to files:
backend/apps/mentorship/api/internal/mutations/program.py
📚 Learning: 2025-07-11T15:57:56.648Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/queries/module.py:39-50
Timestamp: 2025-07-11T15:57:56.648Z
Learning: In the OWASP Nest mentorship GraphQL queries, Strawberry GraphQL automatically converts between Django Module instances and ModuleNode types, so methods can return Module instances directly without manual conversion even when typed as ModuleNode.
Applied to files:
backend/apps/mentorship/api/internal/nodes/program.py
🧬 Code graph analysis (7)
backend/apps/mentorship/admin/program.py (1)
backend/apps/mentorship/models/program_admin.py (1)
ProgramAdmin(10-50)
backend/apps/mentorship/admin/module.py (2)
backend/apps/mentorship/models/mentor_module.py (1)
MentorModule(11-42)backend/apps/mentorship/models/module.py (1)
Module(17-99)
backend/apps/mentorship/api/internal/queries/program.py (3)
backend/apps/mentorship/models/program_admin.py (1)
ProgramAdmin(10-50)backend/apps/mentorship/models/program.py (1)
Program(18-89)backend/apps/mentorship/api/internal/nodes/program.py (1)
admins(34-44)
backend/apps/mentorship/models/__init__.py (2)
backend/apps/mentorship/admin/program.py (1)
ProgramAdmin(17-34)backend/apps/mentorship/models/program_admin.py (1)
ProgramAdmin(10-50)
backend/apps/mentorship/models/program_admin.py (3)
backend/apps/common/models.py (1)
TimestampedModel(37-46)backend/apps/mentorship/admin/program.py (1)
ProgramAdmin(17-34)backend/apps/mentorship/models/program.py (1)
Meta(21-23)
backend/apps/mentorship/api/internal/nodes/program.py (1)
backend/apps/mentorship/models/program_admin.py (1)
ProgramAdmin(10-50)
backend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.py (2)
backend/apps/mentorship/api/internal/nodes/program.py (1)
admins(34-44)backend/apps/mentorship/api/internal/nodes/mentor.py (1)
name(18-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (16)
backend/apps/mentorship/admin/program.py (2)
6-14: LGTM! Clean inline admin configuration.The ProgramAdminInline is properly configured with autocomplete for the user field, which will improve UX when managing program admins. The import aliasing avoids naming conflicts with the ProgramAdmin admin class.
34-34: Inline approach appropriate for through-model management.Using an inline for the ProgramAdmin through model allows managing the role field and provides a better UX compared to the previous horizontal filter approach.
backend/apps/mentorship/admin/module.py (1)
34-34: LGTM! Inline configuration enables better mentor management.The inline approach allows managing mentor-module relationships directly within the Module admin interface.
backend/apps/mentorship/models/__init__.py (1)
9-9: LGTM! ProgramAdmin model properly exported.The new ProgramAdmin through model is now accessible via the mentorship.models namespace.
backend/apps/mentorship/models/program.py (1)
68-74: LGTM! Admins field correctly refactored to user-based model.The admins field now correctly references
nest.Userthrough theProgramAdminmodel. The through model enables storing role information (like OWNER) and therelated_nameprovides a clear reverse relationship.backend/apps/mentorship/api/internal/mutations/program.py (4)
55-55: LGTM! Correct admin ownership assignment.Creating a
ProgramAdminentry withAdminRole.OWNERfor the program creator is the correct approach. This ensures the creator has admin privileges via the new user-based admin model.
79-86: LGTM! Efficient admin permission check.The admin check correctly uses
program.admins.filter(id=user.id).exists()which is efficient and aligns with the new user-based admin model.
136-137: LGTM! Consistent admin permission check.The admin verification follows the same efficient pattern as
update_program, correctly checking user membership in the program's admins.
9-9: No issues found —resolve_users_from_loginsis correctly defined and properly returns a set ofnest.Userinstances compatible withprogram.admins.set().The function exists at
backend/apps/mentorship/api/internal/mutations/module.py:41with signatureresolve_users_from_logins(logins: list[str]) -> set, building and returning a set of User objects that is properly consumed by the ManyToMany relationship setter at line 118.backend/apps/mentorship/api/internal/nodes/program.py (1)
36-44: Admins without linked GitHub accounts are silently excluded.The filter
user__github_user_id__isnull=Falseexcludes program admins whosenest.Userdoesn't have a linked GitHub account. If this is intentional (e.g., the UI requires GitHub data), consider adding a comment documenting this behavior. Otherwise, you may want to return all admins and handle missing GitHub data at the presentation layer.backend/apps/mentorship/api/internal/mutations/module.py (4)
106-107: LGTM!The admin permission check correctly uses the new
program.adminsM2M relationship to verify the user is an admin.
160-161: Program admins cannot assign issues to users.The permission check only allows mentors of this module. Consider whether program admins should also be able to assign issues. If so, add an admin check similar to
update_module.
339-344: LGTM!The permission check correctly allows either program admins or module mentors to edit the module, and the docstring accurately reflects this behavior.
41-60: Function is used and data integrity concern is incorrect.
resolve_users_from_loginsis imported and used inprogram.py(line 117) to resolve admin logins. Additionally, the function only assignsgithub_userif missing (line 52:if not nest_user.github_user), not overwriting existing values. This design allows users to be assigned before they've signed into the system, which is intentional.backend/apps/mentorship/models/program_admin.py (1)
10-50: LGTM!The model is well-structured with appropriate ForeignKey relationships, CASCADE deletes, and a role field that allows for future expansion. The
__str__method provides useful debugging output.backend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.py (1)
41-104: LGTM on migration operation ordering.The operations are correctly ordered: create the through model, migrate data while the old relation exists, remove the old field, then add the new M2M field with the through model.
backend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.py
Outdated
Show resolved
Hide resolved
backend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docker-compose/local/compose.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/module.py:28-29
Timestamp: 2025-07-16T13:49:58.648Z
Learning: In the OWASP Nest mentorship system, mentors can be created with only github_user initially (without nest_user) when assigned to modules. This allows mentors to be assigned before they've signed into the system. When these users eventually sign in, the nest_user is automatically linked to the existing github_user, creating a complete mentor profile. This design provides flexibility in mentor assignment workflows.
📚 Learning: 2025-12-26T06:08:58.549Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 3041
File: .github/workflows/run-ci-cd.yaml:233-243
Timestamp: 2025-12-26T06:08:58.549Z
Learning: Ensure Redis image versions stay in sync across all environments by updating every relevant YAML file together (docker-compose files and CI/CD workflow configurations). When upgrading Redis, bump the image version in all docker-compose files and in all CI workflow YAMLs in one coordinated change to maintain parity across production, staging, local, E2E, and CI tests.
Applied to files:
docker-compose/local/compose.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @backend/apps/mentorship/api/internal/mutations/module.py:
- Around line 42-59: Update the function signature of resolve_users_from_logins
to return set[User] instead of raw set, and change the user-resolution logic to
avoid silently overwriting an existing User.github_user: after
retrieving/creating the User via
User.objects.get_or_create(username=login.lower(), defaults={"github_user":
github_user}), if nest_user.github_user exists and nest_user.github_user !=
github_user then raise a ValueError (or another explicit exception) indicating a
conflicting github_user for that username instead of assigning and saving; only
set and save nest_user.github_user when the user was just created or when it is
currently None. Ensure you still catch GithubUser.DoesNotExist for
GithubUser.objects.get(login__iexact=login.lower()) and include the same error
handling.
🧹 Nitpick comments (2)
backend/apps/mentorship/models/program_admin.py (2)
16-19: Consider usingUniqueConstraintinstead ofunique_together.Django recommends using
constraintswithUniqueConstraintinstead ofunique_togetherfor new code, asunique_togethermay be deprecated in the future.🔎 Proposed refactor
class Meta: db_table = "mentorship_program_admins" verbose_name_plural = "Program admins" - unique_together = ("program", "user") + constraints = [ + models.UniqueConstraint( + fields=["program", "user"], + name="unique_program_admin_per_user", + ) + ]
43-50: Consider usingget_role_display()for human-readable output.The
__str__method usesself.rolewhich returns the stored value ("owner"). For a more readable representation, consider usingself.get_role_display()which returns the label ("Owner").🔎 Proposed refactor
- return f"{self.user} - {self.program} ({self.role})" + return f"{self.user} - {self.program} ({self.get_role_display()})"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/apps/mentorship/admin/module.pybackend/apps/mentorship/admin/program.pybackend/apps/mentorship/api/internal/mutations/module.pybackend/apps/mentorship/api/internal/nodes/program.pybackend/apps/mentorship/models/program_admin.py
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
📚 Learning: 2025-07-11T15:57:56.648Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/queries/module.py:39-50
Timestamp: 2025-07-11T15:57:56.648Z
Learning: In the OWASP Nest mentorship GraphQL queries, Strawberry GraphQL automatically converts between Django Module instances and ModuleNode types, so methods can return Module instances directly without manual conversion even when typed as ModuleNode.
Applied to files:
backend/apps/mentorship/api/internal/nodes/program.py
📚 Learning: 2025-07-13T05:55:46.436Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Applied to files:
backend/apps/mentorship/api/internal/nodes/program.py
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/mentorship/api/internal/nodes/program.pybackend/apps/mentorship/admin/module.pybackend/apps/mentorship/models/program_admin.pybackend/apps/mentorship/admin/program.pybackend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/mentorship/api/internal/nodes/program.pybackend/apps/mentorship/admin/module.pybackend/apps/mentorship/models/program_admin.pybackend/apps/mentorship/admin/program.pybackend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-07-16T13:49:58.648Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/module.py:28-29
Timestamp: 2025-07-16T13:49:58.648Z
Learning: In the OWASP Nest mentorship system, mentors can be created with only github_user initially (without nest_user) when assigned to modules. This allows mentors to be assigned before they've signed into the system. When these users eventually sign in, the nest_user is automatically linked to the existing github_user, creating a complete mentor profile. This design provides flexibility in mentor assignment workflows.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-07-31T07:05:25.056Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 0
File: :0-0
Timestamp: 2025-07-31T07:05:25.056Z
Learning: In the OWASP Nest project, Django views may not properly access authenticated users from sessions created by Strawberry GraphQL mutations. The issue occurs because Django's AuthenticationMiddleware doesn't always populate request.user from session data that GraphQL context successfully uses via info.context.request.user. The solution is to manually resolve users from session data using request.session.get('_auth_user_id') and User.objects.select_related('github_user').get(pk=user_id) to match the same authentication mechanism used by GraphQL context.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-08-17T11:55:55.990Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2000
File: backend/apps/mentorship/management/commands/sync_module_issues.py:109-145
Timestamp: 2025-08-17T11:55:55.990Z
Learning: In the OWASP/Nest mentorship system, tasks are designed to be assigned to only one assignee per issue, even if GitHub issues can have multiple assignees. The sync_module_issues command correctly uses issue.assignees.first() to create one task per issue for the first assignee only.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-07-14T16:18:07.287Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/components/ModuleForm.tsx:112-134
Timestamp: 2025-07-14T16:18:07.287Z
Learning: In the OWASP Nest mentorship system, date validation for modules (ensuring start date precedes end date) is handled on the backend in the module GraphQL mutations via the `_validate_module_dates` helper function in backend/apps/mentorship/graphql/mutations/module.py, which prevents invalid date ranges from being stored in the database.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
🧬 Code graph analysis (5)
backend/apps/mentorship/api/internal/nodes/program.py (2)
backend/apps/mentorship/api/internal/nodes/enum.py (2)
ExperienceLevelEnum(12-18)ProgramStatusEnum(22-27)backend/apps/mentorship/models/program_admin.py (1)
ProgramAdmin(10-50)
backend/apps/mentorship/admin/module.py (2)
backend/apps/mentorship/models/mentor_module.py (1)
MentorModule(11-42)backend/apps/mentorship/models/module.py (1)
Module(17-99)
backend/apps/mentorship/models/program_admin.py (3)
backend/apps/common/models.py (1)
TimestampedModel(37-46)backend/apps/mentorship/admin/program.py (1)
ProgramAdmin(17-31)backend/apps/mentorship/models/program.py (1)
Meta(21-23)
backend/apps/mentorship/admin/program.py (1)
backend/apps/mentorship/models/program_admin.py (1)
ProgramAdmin(10-50)
backend/apps/mentorship/api/internal/mutations/module.py (3)
backend/apps/mentorship/api/internal/nodes/program.py (1)
admins(34-43)backend/apps/mentorship/models/mentor.py (1)
Mentor(11-40)backend/apps/mentorship/api/internal/queries/mentorship.py (1)
is_mentor(36-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (7)
backend/apps/mentorship/admin/module.py (1)
9-22: LGTM!The
MentorModuleInlineis correctly configured withautocomplete_fieldsfor efficient mentor selection and properly integrated intoModuleAdmin. The inline allows managing mentor assignments directly from the Module admin page.backend/apps/mentorship/api/internal/mutations/module.py (4)
105-106: LGTM!The permission check correctly validates that the current user is a program admin before allowing module creation. Using
.exists()is efficient for this check.
159-160: LGTM!The mentor permission checks are consistent across all issue/task mutations. The check
Mentor.objects.filter(nest_user=user, modules=module).exists()correctly validates that the user is a mentor of the specific module, not just any module.Also applies to: 198-199, 235-236, 294-295
338-343: LGTM!The permission model correctly allows both program admins and module mentors to update modules. This provides appropriate flexibility while maintaining access control.
42-59: No action needed. This function is exported and used inbackend/apps/mentorship/api/internal/mutations/program.py(line 117) for resolving admin logins during program mutations.backend/apps/mentorship/admin/program.py (1)
6-20: LGTM!The import alias
ProgramAdminThroughModelcorrectly avoids the name collision with theProgramAdminModelAdmin class. The inline configuration withautocomplete_fields=("user",)provides an efficient admin interface for managing program administrators through the new through-model relationship.backend/apps/mentorship/api/internal/nodes/program.py (1)
33-43: No changes needed—the code is correctly typed.The return type
list[UserNode]is correct.UserNodeis registered as the Strawberry type forapps.github.models.user.User(imported asGithubUserin this file), and strawberry-django automatically converts the returned Django model instances toUserNodeobjects. There is no type mismatch.The secondary concern about excluding admins without linked GitHub accounts is a design choice. If this behavior is intentional, the code is fine as-is.
kasya
force-pushed
the
update-program-admins
branch
from
8d5adee to
8aab4c9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/apps/mentorship/api/internal/mutations/module.py (2)
228-228: Update docstring to match actual permission requirements.The docstring states "User must be a mentor and an admin of the program," but the actual permission check at lines 239-240 only requires the user to be a mentor of the specific module. The docstring should be updated to reflect the actual requirement.
🔎 Proposed docstring fix
- """Set a deadline for a task. User must be a mentor and an admin of the program.""" + """Set a deadline for a task. User must be a mentor of this module."""
287-287: Update docstring to match actual permission requirements.Similar to
set_task_deadline, the docstring states "User must be a mentor and an admin of the program," but the actual permission check at lines 298-299 only requires the user to be a mentor of the specific module.🔎 Proposed docstring fix
- """Clear the deadline for a task. User must be a mentor and an admin of the program.""" + """Clear the deadline for a task. User must be a mentor of this module."""
🧹 Nitpick comments (1)
backend/apps/mentorship/api/internal/mutations/module.py (1)
331-331: Consider making the docstring more descriptive.The docstring could be more explicit about permission requirements. While the error message at line 345 clearly states who can edit modules, the docstring would benefit from including this information upfront.
🔎 Suggested docstring improvement
- """Update an existing mentorship module.""" + """Update an existing mentorship module. User must be an admin of the program or a mentor of this module."""
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/apps/mentorship/api/internal/mutations/module.pydocker-compose/local/compose.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- docker-compose/local/compose.yaml
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/module.py:28-29
Timestamp: 2025-07-16T13:49:58.648Z
Learning: In the OWASP Nest mentorship system, mentors can be created with only github_user initially (without nest_user) when assigned to modules. This allows mentors to be assigned before they've signed into the system. When these users eventually sign in, the nest_user is automatically linked to the existing github_user, creating a complete mentor profile. This design provides flexibility in mentor assignment workflows.
📚 Learning: 2025-07-16T13:49:58.648Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/module.py:28-29
Timestamp: 2025-07-16T13:49:58.648Z
Learning: In the OWASP Nest mentorship system, mentors can be created with only github_user initially (without nest_user) when assigned to modules. This allows mentors to be assigned before they've signed into the system. When these users eventually sign in, the nest_user is automatically linked to the existing github_user, creating a complete mentor profile. This design provides flexibility in mentor assignment workflows.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-07-31T07:05:25.056Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 0
File: :0-0
Timestamp: 2025-07-31T07:05:25.056Z
Learning: In the OWASP Nest project, Django views may not properly access authenticated users from sessions created by Strawberry GraphQL mutations. The issue occurs because Django's AuthenticationMiddleware doesn't always populate request.user from session data that GraphQL context successfully uses via info.context.request.user. The solution is to manually resolve users from session data using request.session.get('_auth_user_id') and User.objects.select_related('github_user').get(pk=user_id) to match the same authentication mechanism used by GraphQL context.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-07-14T16:18:07.287Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/components/ModuleForm.tsx:112-134
Timestamp: 2025-07-14T16:18:07.287Z
Learning: In the OWASP Nest mentorship system, date validation for modules (ensuring start date precedes end date) is handled on the backend in the module GraphQL mutations via the `_validate_module_dates` helper function in backend/apps/mentorship/graphql/mutations/module.py, which prevents invalid date ranges from being stored in the database.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-08-17T11:55:55.990Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2000
File: backend/apps/mentorship/management/commands/sync_module_issues.py:109-145
Timestamp: 2025-08-17T11:55:55.990Z
Learning: In the OWASP/Nest mentorship system, tasks are designed to be assigned to only one assignee per issue, even if GitHub issues can have multiple assignees. The sync_module_issues command correctly uses issue.assignees.first() to create one task per issue for the first assignee only.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
🔇 Additional comments (3)
backend/apps/mentorship/api/internal/mutations/module.py (3)
109-110: Permission check correctly updated to use program admins.The permission logic now correctly checks if the user is in
program.admins, aligning with the PR's objective to change admins from a Mentor-based relation to a User-based many-to-many relation.
163-164: Permission check correctly restricts to module mentors.The permission logic appropriately restricts issue assignment to mentors of the specific module, using the new
nest_userfield on Mentor.
202-203: Permission check correctly restricts to module mentors.Consistent with the
assign_issue_to_usermutation, this correctly restricts issue unassignment to mentors of the specific module.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @backend/apps/mentorship/api/internal/mutations/module.py:
- Around line 319-324: The admin check in the module edit guard is comparing
Admin.id to User.id and always false; change the lookup to match the admin's
user relation (like Mentor) — e.g., replace
program.admins.filter(id=user.id).exists() with a check that compares the Admin
model's user field (e.g., program.admins.filter(nest_user=user).exists() or
program.admins.filter(user_id=user.id).exists()) so is_admin correctly detects
program admins (same fix as in create_module).
- Around line 87-88: The permission check is comparing program.admins' Admin PKs
to a User PK (filter(id=user.id)) which always fails; update the check in
create_module to filter by the Admin's nest_user relation instead (e.g.,
program.admins.filter(nest_user_id=user.id).exists() or
program.admins.filter(nest_user=user).exists()) so the correct admin-user link
is tested before raising PermissionDenied.
In @backend/apps/mentorship/models/common/linked_user.py:
- Around line 13-25: The OneToOneField definitions github_user and nest_user in
the LinkedUser model lack explicit related_name values, causing reverse-relation
conflicts when multiple concrete models inherit this abstract base; update both
OneToOneField declarations (github_user and nest_user) to include
related_name="%(class)s_<field>" (or at least related_name="%(class)s") so
Django generates unique reverse names per concrete subclass and avoids system
check errors.
In @docker-compose/local/compose.yaml:
- Line 69: Revert the feature-branch specific Docker volume name back to the
standard name: find every occurrence of the volume identifier
"db-data-mentorship-admin" in the compose file and replace it with "db-data"
(this includes the service volume mount entry referencing
/var/lib/postgresql/data and the matching volumes definition); ensure the volume
name is consistently "db-data" so main branch uses the shared, stable local
volume name.
🧹 Nitpick comments (6)
backend/apps/mentorship/admin/module.py (1)
5-16: Inline looks correct, but consider exposingrole(and fix MentorModule.mentor verbose_name) to avoid confusing admin UX.
MentorModuleInlineis a good fit for managing theModule.mentorsthrough table.- Right now the inline hides
MentorModule.role; if that field is meant to be editable, include it infields.- Because this change surfaces the inline in admin, any mislabeling becomes user-visible:
MentorModule.mentorcurrently hasverbose_name="Program"(inbackend/apps/mentorship/models/mentor_module.py), which will read oddly in the inline—worth correcting.Proposed tweak (if role should be editable)
class MentorModuleInline(admin.TabularInline): @@ - fields = ("mentor",) + fields = ("mentor", "role")backend/apps/mentorship/admin/admin.py (1)
8-16: Consider showing GitHub login directly in list display.The current
list_displayshows thegithub_userobject representation. For better readability in the admin list view, consider displaying the login and optional nest user directly.💡 Suggested enhancement
class AdminAdmin(admin.ModelAdmin): """Admin view for Admin model.""" - list_display = ("github_user",) + list_display = ("id", "github_user__login", "nest_user") search_fields = ( "github_user__login", "github_user__name", )This makes it easier to scan the admin list and see which admins have linked Nest users.
backend/apps/mentorship/api/internal/mutations/program.py (1)
25-44: Extract nest_user linking logic to reduce duplication.The logic for linking
nest_userto anAdmin(lines 32-38) is duplicated in thecreate_programmutation (lines 79-81). Consider extracting this to a helper method on theAdminmodel or a utility function.♻️ Suggested refactor
Create a helper method to ensure the nest_user link:
# In apps/mentorship/models/admin.py or a utils module def ensure_nest_user_link(admin: Admin) -> None: """Link nest_user to admin if not already linked.""" if not admin.nest_user: try: from apps.nest.models import User nest_user = User.objects.get(github_user=admin.github_user) admin.nest_user = nest_user admin.save(update_fields=["nest_user"]) except User.DoesNotExist: passThen use it in both locations:
admin, _ = Admin.objects.get_or_create(github_user=github_user) -if not admin.nest_user: - try: - nest_user = User.objects.get(github_user=github_user) - admin.nest_user = nest_user - admin.save(update_fields=["nest_user"]) - except User.DoesNotExist: - pass +ensure_nest_user_link(admin)This eliminates duplication and centralizes the linking logic.
backend/apps/mentorship/api/internal/queries/program.py (1)
78-82: Minor inefficiency: redundant query on prefetched relation.
program.admins.filter(id=admin.id).exists()executes a new query even thoughadminsis prefetched. Use the in-memory prefetched data instead.♻️ Suggested optimization
results = [] for program in paginated_programs: - is_admin = admin and program.admins.filter(id=admin.id).exists() + is_admin = admin and any(a.id == admin.id for a in program.admins.all()) program.user_role = "admin" if is_admin else "mentor" results.append(program)backend/apps/mentorship/models/program_admin.py (1)
16-19: Consider usingUniqueConstraintfor modern Django style.
unique_togetheris a legacy option. Django recommendsUniqueConstraintinMeta.constraintsfor new code.♻️ Modern constraint style
class Meta: db_table = "mentorship_program_admins" verbose_name_plural = "Program admins" - unique_together = ("program", "admin") + constraints = [ + models.UniqueConstraint(fields=["program", "admin"], name="unique_program_admin"), + ]backend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.py (1)
188-191: Consider adding a warning log for irreversible migration.Since the reverse is a no-op, adding a logged warning in a reverse function would alert operators during rollback attempts.
♻️ Optional: Add reverse warning
+def warn_irreversible(apps, schema_editor): + """Warn that this migration cannot be fully reversed.""" + logger.warning( + "Migration 0007 reverse: ProgramAdmin data was not restored. " + "Manual intervention may be required." + ) + + class Migration(migrations.Migration): # ... operations = [ # ... migrations.RunPython( migrate_program_admins_to_admin_model, - migrations.RunPython.noop, + warn_irreversible, ),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/moduleQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/programsMutations.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/programsQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (17)
backend/apps/mentorship/admin/__init__.pybackend/apps/mentorship/admin/admin.pybackend/apps/mentorship/admin/module.pybackend/apps/mentorship/admin/program.pybackend/apps/mentorship/api/internal/mutations/module.pybackend/apps/mentorship/api/internal/mutations/program.pybackend/apps/mentorship/api/internal/nodes/admin.pybackend/apps/mentorship/api/internal/nodes/program.pybackend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.pybackend/apps/mentorship/models/__init__.pybackend/apps/mentorship/models/admin.pybackend/apps/mentorship/models/common/__init__.pybackend/apps/mentorship/models/common/linked_user.pybackend/apps/mentorship/models/program.pybackend/apps/mentorship/models/program_admin.pydocker-compose/local/compose.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/apps/mentorship/admin/program.py
- backend/apps/mentorship/models/init.py
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
📚 Learning: 2025-07-11T15:57:56.648Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/queries/module.py:39-50
Timestamp: 2025-07-11T15:57:56.648Z
Learning: In the OWASP Nest mentorship GraphQL queries, Strawberry GraphQL automatically converts between Django Module instances and ModuleNode types, so methods can return Module instances directly without manual conversion even when typed as ModuleNode.
Applied to files:
backend/apps/mentorship/api/internal/nodes/admin.pybackend/apps/mentorship/api/internal/nodes/program.py
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/mentorship/api/internal/nodes/admin.pybackend/apps/mentorship/models/admin.pybackend/apps/mentorship/admin/admin.pybackend/apps/mentorship/api/internal/mutations/program.pybackend/apps/mentorship/models/common/linked_user.pybackend/apps/mentorship/admin/module.pybackend/apps/mentorship/admin/__init__.pybackend/apps/mentorship/models/common/__init__.pybackend/apps/mentorship/models/program.pybackend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/api/internal/nodes/program.pybackend/apps/mentorship/api/internal/mutations/module.pybackend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.pybackend/apps/mentorship/models/program_admin.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/mentorship/api/internal/nodes/admin.pybackend/apps/mentorship/models/admin.pybackend/apps/mentorship/admin/admin.pybackend/apps/mentorship/api/internal/mutations/program.pybackend/apps/mentorship/models/common/linked_user.pybackend/apps/mentorship/admin/module.pybackend/apps/mentorship/admin/__init__.pybackend/apps/mentorship/models/common/__init__.pybackend/apps/mentorship/models/program.pybackend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/api/internal/nodes/program.pybackend/apps/mentorship/api/internal/mutations/module.pybackend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.pybackend/apps/mentorship/models/program_admin.py
📚 Learning: 2026-01-05T16:20:47.532Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 3205
File: docker-compose/local.yaml:32-32
Timestamp: 2026-01-05T16:20:47.532Z
Learning: In the OWASP/Nest repository, feature branches use unique volume name suffixes in docker-compose files to prevent volume clashes across parallel development efforts. For example, the feature/nest-zappa-migration branch uses `-zappa` suffix for all volume names (backend-venv-zappa, cache-data-zappa, db-data-zappa, etc.) to ensure isolated environments when switching between branches.
Applied to files:
docker-compose/local/compose.yaml
📚 Learning: 2025-12-26T06:08:58.549Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 3041
File: .github/workflows/run-ci-cd.yaml:233-243
Timestamp: 2025-12-26T06:08:58.549Z
Learning: Ensure Redis image versions stay in sync across all environments by updating every relevant YAML file together (docker-compose files and CI/CD workflow configurations). When upgrading Redis, bump the image version in all docker-compose files and in all CI workflow YAMLs in one coordinated change to maintain parity across production, staging, local, E2E, and CI tests.
Applied to files:
docker-compose/local/compose.yaml
📚 Learning: 2025-07-13T05:55:46.436Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Applied to files:
backend/apps/mentorship/api/internal/mutations/program.pybackend/apps/mentorship/api/internal/nodes/program.py
📚 Learning: 2025-07-08T17:24:36.501Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:41-44
Timestamp: 2025-07-08T17:24:36.501Z
Learning: In the mentorship program GraphQL mutations, date validation is handled at the GraphQL schema/node level in the input types (CreateProgramInput, UpdateProgramInput), preventing null values from reaching the mutation logic where date comparisons are performed.
Applied to files:
backend/apps/mentorship/api/internal/mutations/program.py
📚 Learning: 2025-07-16T13:49:58.648Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/module.py:28-29
Timestamp: 2025-07-16T13:49:58.648Z
Learning: In the OWASP Nest mentorship system, mentors can be created with only github_user initially (without nest_user) when assigned to modules. This allows mentors to be assigned before they've signed into the system. When these users eventually sign in, the nest_user is automatically linked to the existing github_user, creating a complete mentor profile. This design provides flexibility in mentor assignment workflows.
Applied to files:
backend/apps/mentorship/api/internal/mutations/program.pybackend/apps/mentorship/models/common/linked_user.pybackend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
backend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
backend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
backend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-07-14T16:18:07.287Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/components/ModuleForm.tsx:112-134
Timestamp: 2025-07-14T16:18:07.287Z
Learning: In the OWASP Nest mentorship system, date validation for modules (ensuring start date precedes end date) is handled on the backend in the module GraphQL mutations via the `_validate_module_dates` helper function in backend/apps/mentorship/graphql/mutations/module.py, which prevents invalid date ranges from being stored in the database.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-07-31T07:05:25.056Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 0
File: :0-0
Timestamp: 2025-07-31T07:05:25.056Z
Learning: In the OWASP Nest project, Django views may not properly access authenticated users from sessions created by Strawberry GraphQL mutations. The issue occurs because Django's AuthenticationMiddleware doesn't always populate request.user from session data that GraphQL context successfully uses via info.context.request.user. The solution is to manually resolve users from session data using request.session.get('_auth_user_id') and User.objects.select_related('github_user').get(pk=user_id) to match the same authentication mechanism used by GraphQL context.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
📚 Learning: 2025-08-17T11:55:55.990Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2000
File: backend/apps/mentorship/management/commands/sync_module_issues.py:109-145
Timestamp: 2025-08-17T11:55:55.990Z
Learning: In the OWASP/Nest mentorship system, tasks are designed to be assigned to only one assignee per issue, even if GitHub issues can have multiple assignees. The sync_module_issues command correctly uses issue.assignees.first() to create one task per issue for the first assignee only.
Applied to files:
backend/apps/mentorship/api/internal/mutations/module.py
🧬 Code graph analysis (10)
backend/apps/mentorship/api/internal/nodes/admin.py (2)
frontend/src/types/__generated__/graphql.ts (1)
AdminNode(21-27)backend/apps/owasp/api/internal/nodes/member_snapshot.py (1)
github_user(32-34)
backend/apps/mentorship/models/admin.py (5)
backend/apps/common/models.py (1)
TimestampedModel(37-46)backend/apps/mentorship/models/common/experience_level.py (1)
ExperienceLevel(6-25)backend/apps/mentorship/models/common/linked_user.py (2)
LinkedUser(6-25)Meta(9-10)backend/apps/mentorship/models/common/matching_attributes.py (1)
MatchingAttributes(6-24)backend/apps/mentorship/models/program.py (1)
Meta(21-23)
backend/apps/mentorship/admin/admin.py (1)
backend/apps/mentorship/models/admin.py (1)
Admin(13-27)
backend/apps/mentorship/models/common/linked_user.py (4)
backend/apps/mentorship/models/admin.py (1)
Meta(16-18)backend/apps/mentorship/models/program.py (1)
Meta(21-23)backend/apps/mentorship/models/program_admin.py (1)
Meta(16-19)backend/apps/owasp/api/internal/nodes/member_snapshot.py (1)
github_user(32-34)
backend/apps/mentorship/admin/module.py (2)
backend/apps/mentorship/models/mentor_module.py (1)
MentorModule(11-42)backend/apps/mentorship/models/module.py (1)
Module(17-99)
backend/apps/mentorship/admin/__init__.py (1)
backend/apps/mentorship/admin/admin.py (1)
AdminAdmin(8-16)
backend/apps/mentorship/models/common/__init__.py (1)
backend/apps/mentorship/models/common/linked_user.py (1)
LinkedUser(6-25)
backend/apps/mentorship/api/internal/nodes/program.py (2)
backend/apps/mentorship/api/internal/nodes/admin.py (1)
AdminNode(7-25)frontend/src/types/__generated__/graphql.ts (1)
AdminNode(21-27)
backend/apps/mentorship/api/internal/mutations/module.py (5)
backend/apps/mentorship/api/internal/nodes/program.py (1)
admins(32-34)backend/tests/apps/owasp/management/commands/owasp_aggregate_contributions_test.py (1)
filter(28-30)backend/tests/apps/owasp/management/commands/owasp_aggregate_projects_test.py (1)
exists(61-62)backend/apps/mentorship/models/mentor.py (1)
Mentor(11-40)backend/apps/mentorship/api/internal/queries/mentorship.py (1)
is_mentor(36-48)
backend/apps/mentorship/models/program_admin.py (4)
backend/apps/common/models.py (1)
TimestampedModel(37-46)backend/apps/mentorship/models/admin.py (1)
Meta(16-18)backend/apps/mentorship/models/common/linked_user.py (1)
Meta(9-10)backend/apps/mentorship/models/program.py (1)
Meta(21-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (21)
backend/apps/mentorship/admin/module.py (1)
34-35: Good wiring of the through-inline onto ModuleAdmin.
inlines = (MentorModuleInline,)should make mentor assignments manageable directly from the Module admin page as intended.backend/apps/mentorship/models/common/__init__.py (1)
1-4: Good: exportingLinkedUserfor shared model inheritance.Makes sense given the new Admin/ProgramAdmin modeling.
backend/apps/mentorship/admin/__init__.py (1)
3-3: The Admin model is properly registered. The import in__init__.pywill load the admin.py module, which executesadmin.site.register(Admin, AdminAdmin)at line 19, correctly registering it in the Django admin.backend/apps/mentorship/models/program.py (1)
68-74: The review comment's concern about.admins.add()is not applicable—theProgramAdmin.rolefield has a default value, so Django M2M operations work as expected.The
ProgramAdminthrough model only has two ForeignKeys (programandadmin), aroleCharField withdefault=AdminRole.OWNER, and auto-managed timestamp fields fromTimestampedModel. Sincerolehas a default,program.admins.add(admin)will function normally without requiring directProgramAdmininstantiation.Likely an incorrect or invalid review comment.
backend/apps/mentorship/api/internal/nodes/admin.py (1)
1-25:AdminNodeis properly optimized; the null-handling is defensive and acceptable given the non-nullable constraint.The N+1 concern is not applicable here. When
AdminNodeis resolved throughProgramNode.admins(), theProgramobjects are fetched withprefetch_related("admins__github_user")(visible in bothget_programandmy_programsqueries). DirectAdmin.objectsqueries in the mutations are used only for permission checks and do not triggerAdminNodefield resolution.Regarding null-handling:
LinkedUser.github_useris indeed a required OneToOneField (non-nullable at the DB level), so returning""is defensive programming that shouldn't encounter null values in practice. TheAdmin.__str__()method also assumesgithub_useris always present without a fallback. The current approach is reasonable.Likely an incorrect or invalid review comment.
backend/apps/mentorship/models/common/linked_user.py (1)
6-10: LGTM: Abstract model structure.The abstract base model structure is correct and follows Django best practices.
backend/apps/mentorship/models/admin.py (2)
1-18: LGTM: Admin model structure.The model correctly inherits from multiple mixins and defines appropriate metadata. The multiple inheritance pattern follows Django best practices.
20-27: LGTM: String representation.The
__str__method appropriately returns the GitHub username. Sincegithub_useris a required field, this should not raiseAttributeErrorunder normal circumstances.backend/apps/mentorship/api/internal/nodes/program.py (2)
7-7: LGTM: Import updated for Admin-based model.The import correctly references the new
AdminNodetype.
32-34: LGTM: Admin resolution logic.The resolver correctly returns a list of
AdminNodeorNonefor empty results. The eager loading vialist()is acceptable given that programs typically have a small number of admins.backend/apps/mentorship/admin/admin.py (1)
19-19: LGTM: Admin registration.The model is correctly registered with the Django admin site.
backend/apps/mentorship/api/internal/mutations/program.py (5)
9-20: LGTM: Imports updated for Admin-based workflow.The imports correctly reflect the migration from Mentor-based to Admin-based program administration.
78-84: LGTM: Admin creation for program owner.The logic correctly creates or retrieves an
Adminfor the program creator and establishes them as the owner viaProgramAdmin. The nest_user linking ensures the admin record is properly connected.
108-116: LGTM: Admin authorization check.The authorization correctly verifies that the user has an associated
Adminrecord and is listed as an admin for the program. The defensive use of.filter().first()appropriately handles users without admin records.
146-148: LGTM: Admin assignment logic.The admin assignment correctly uses
resolve_admins_from_loginsto resolve and set program administrators.
166-168: LGTM: Consistent authorization.The status update mutation maintains the same authorization pattern, ensuring only program admins can modify the status.
backend/apps/mentorship/api/internal/queries/program.py (1)
47-56: LGTM! Clean transition to Admin-based access control.The logic correctly:
- Looks up the Admin record for the current user
- Retrieves program IDs via ProgramAdmin through-model
- Combines with mentor-based access using Q objects
backend/apps/mentorship/api/internal/mutations/module.py (2)
29-41: LGTM! Clean helper function for mentor resolution.The function correctly handles GitHub user lookup with case-insensitive matching and properly raises a descriptive error when a user is not found.
141-142: LGTM! Mentor permission checks are correct.These checks correctly verify that the current user is a mentor of the specific module via
Mentor.objects.filter(nest_user=user, modules=module).exists().Also applies to: 180-181, 217-218, 276-277
backend/apps/mentorship/models/program_admin.py (1)
10-50: LGTM! Well-structured through model.The model correctly establishes the M2M relationship with role support. The
CASCADEdelete behavior, related names, and__str__implementation are all appropriate.backend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.py (1)
75-206: LGTM! Migration operations are correctly ordered.The sequence properly:
- Creates Admin model first
- Creates ProgramAdmin through model
- Runs data migration while old field exists
- Removes old admins field
- Adds new through-based M2M field
This ensures data is migrated before the old relationship is removed.
backend/apps/mentorship/migrations/0007_programadmin_alter_program_admins.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@backend/apps/mentorship/migrations/0007_alter_mentor_github_user_alter_mentor_nest_user_and_more.py`:
- Around line 20-61: The migration currently skips nest_user entries with no
github_user_id (via checks on nest_user.github_user_id), which will drop those
admins when Program.admins is removed; update the migration to detect any
program.admins members lacking github_user_id before making changes and abort
with a clear MigrationError listing affected program ids/nest_user ids (or
instruct pre-linking) instead of silently skipping; specifically add a pre-check
iterating program.admins to collect problematic nest_user ids, and if any exist
raise django.db.migrations.exceptions.MigrationError with a descriptive message
so admin_model/program_admin_model bulk operations never proceed.
♻️ Duplicate comments (2)
backend/apps/mentorship/api/internal/mutations/program.py (2)
25-44: Avoid silent swallow when linking admins to Nest users.The
User.DoesNotExistbranch is still empty; a short comment/log would make the intended behavior explicit and observable.
145-147: Adminsset()still risks removing existing owners.Replacing the full admin set can remove the creator/current owner if not included in
admin_logins. Please preserve OWNERs or enforce a non-empty admin set.
🧹 Nitpick comments (1)
backend/apps/mentorship/api/internal/queries/program.py (1)
11-11: Optional: avoid per-program admin lookup queries.Since admins are prefetched,
program.admins.filter(...).exists()triggers extra queries per program. You can computeis_adminin memory to avoid N+1 overhead.♻️ Possible refinement
- is_admin = program.admins.filter(nest_user=user).exists() + is_admin = any( + admin.nest_user_id == user.id for admin in program.admins.all() + )Also applies to: 26-28, 47-62, 77-78
.../apps/mentorship/migrations/0007_alter_mentor_github_user_alter_mentor_nest_user_and_more.py
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3185 +/- ##
==========================================
- Coverage 96.78% 96.73% -0.06%
==========================================
Files 512 517 +5
Lines 15862 15914 +52
Branches 2130 2129 -1
==========================================
+ Hits 15352 15394 +42
- Misses 421 430 +9
- Partials 89 90 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/apps/mentorship/api/internal/mutations/program.py (1)
111-118:⚠️ Potential issue | 🟠 MajorAdmin nest_user linkage not guaranteed on login; permission checks will fail.
The Admin model inherits from LinkedUser, allowing
nest_userto be null. When admins are created viaresolve_admins_from_loginsand no corresponding nest.User exists yet,admin.nest_userremains unset. Later, when the user signs in viaauthenticate_via_github, a nest.User is created with the github_user, but there is no automatic signal or handler linking the existing Admin object'snest_userfield. Consequently, the permission checks at lines 111 and 168 that filter byadmin__nest_user=userwill fail for these admins.The
resolve_admins_from_loginsfunction attempts to link nest_user after User lookup, but it is only called when explicitly updating program admin logins (line 149), not during user authentication. There is no post-login signal handler that backtracks to update Admin objects with the newly-created nest.User.Recommendation: Either add a signal handler on nest.User creation to backlink Admin/Mentor objects with matching github_user, or modify the permission checks to match by
github_useras a fallback whennest_useris null.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/apps/mentorship/api/internal/mutations/module.py`:
- Around line 143-144: The PermissionDenied exceptions are being constructed
with a keyword argument (e.g. PermissionDenied(msg="Only mentors of this module
can assign issues.")) which will raise TypeError; update every
PermissionDenied(...) call in this file to pass the message as a positional
argument instead (e.g. PermissionDenied("Only mentors of this module can assign
issues.")), including the one after the Mentor.objects.filter(nest_user=user,
modules=module).exists() check and the other four call sites in this module.py
file so all PermissionDenied constructions use positional messages.
In `@backend/apps/mentorship/api/internal/mutations/program.py`:
- Around line 148-150: The current use of program.admins.set(admins_to_set) will
replace all ProgramAdmin relations and can remove the current user (created in
create_program), causing a lockout; instead, ensure the current user remains an
admin or perform explicit CRUD: use
resolve_admins_from_logins(input_data.admin_logins) to compute desired IDs, then
include the current user ID (the user checked in the permission block around
lines 111–118) in that desired set before calling any replace, or alternatively
call ProgramAdmin.create()/program.admins.add() for new admins and
program.admins.remove()/ProgramAdmin.delete() only for admins that should be
revoked (excluding the current user), so that program.admins.set is not used to
remove protected admins.
🧹 Nitpick comments (1)
backend/apps/mentorship/api/internal/mutations/program.py (1)
25-47: Add generic type parameter to return annotation for consistency.
resolve_mentors_from_loginsinmodule.py(line 30) returnsset[Mentor], but this function returns bareset. Useset[Admin]for consistency and type safety.Proposed fix
-def resolve_admins_from_logins(logins: list[str]) -> set: +def resolve_admins_from_logins(logins: list[str]) -> set[Admin]:
|
2 participants
Updated Program Admin relation.
Previously admins were a subset of mentors. They should be a m2m to User instance instead.
Also updated some permissions for managing programs and modules that were affected by this change.
Checklist
make check-testlocally and all tests passed